Adding TracerProviderSdkOptions and use factories for all options #1901
Conversation
|
This is also related to #1889 |
|
@ejsmith I totally get what you are thinking with this, and I tried similar things, but I don't think it really solves anything. We're moving the problem from TracerProviderSdk to an options class. Think about this: What is the difference between Another thing... this seems more like leaking the implementation than providing options. Could you bind this new options class to IConfiguration? I feel like options classes should be ~POCOs. But really, they should be options. Like timeouts, extension points, etc.. If we go with this, is the builder really adding any value? We could just move all the extensions onto the options. And then rename the options builder 🤣 I say this all with ❤️ I really appreciate the collaboration on this. Super helpful! |
|
Thanks for the feedback. I wanted to try and keep this PR small. But I can expand it to show how it would work with the hosting package. Builders in .NET are generally just providing a fluent configuration API around an options object. Generally we are just trying to keep the 2 phases separate and make it so that configuring doesn't force you to construct instances. |
Changes
This PR is creating
TracerProviderSdkOptionsclass and making it so that the builder is not forced to new up any instances of processors or instrumentations during the configuration phase. This PR is trying to work towards a solution to be able to add DI features (#894) to the builder and allow instances to be created from a DI container, but without taking any DI dependencies in the core project. It's also trying to create a separation between the builder phase and the runtime phase by not forcing instances of classed to be created during the builder phase.I am hoping to get feedback and see if this is something that the team is interested in pursuing or not.